ENH: Improve PlaneSegmentation.add_roi doc #1308
Conversation
There was a problem hiding this comment.
IMO I'd prefer image_mask over pixel_mask in all cases. If the image_mask is numerically sparse, gzip should be able to take care of that. image_mask + gzip will probably take up even less space than unzipped pixel_mask, especially if the image_mask data in binary. If you want to go next level, you could also set the fill_value to 0 and only fill in the chunks that contain some of the ROI. I can't think of a good compelling case for using pixel_mask over image_mask
|
@bendichter with regard to storage in the file that is true, but keep in mind that in general, the same optimizations (chunking, compression) are not available in memory. I.e., once a user has to load the data pixel_mask will in general be more efficient for sparse ROI's in memory than image_mask. For small images the difference is probably not important but for large-images full image masks per ROI can become very expensive. |
|
@oruebel ok that's a good point |
|
Also, applying compression to an NWB file is an advanced step that I think many users are not yet doing. If we are going to encourage usage that is optimized for the compressed case, then we should make sure to encourage compression in the tutorials and documentation. I updated the docstring again to be more clear and address the points you raised. Feel free to edit or make further suggestions. |
| weight of 0. A voxel mask is a list of quadruplets or Nx4 array, that is similar to the pixel | ||
| mask, except the format is (x1, y1, z1, weight1). | ||
|
|
||
| If the NWB file will be compressed, then using an image mask will usually result in a smaller |
There was a problem hiding this comment.
Are we suggesting that users compress the entire NWB file? Is there a way of just compressing the image masks?
There was a problem hiding this comment.
Good point. I will add 'data' to the allowed types for the masks and test this out.
Codecov Report
@@ Coverage Diff @@
## dev #1308 +/- ##
==========================================
+ Coverage 83.66% 91.98% +8.32%
==========================================
Files 27 27
Lines 2608 2608
Branches 681 681
==========================================
+ Hits 2182 2399 +217
+ Misses 339 134 -205
+ Partials 87 75 -12
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 13 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Motivation
The docstring for
PlaneSegmentation.add_roi(...)should describe why/when to use image_mask vs pixel_mask or voxel_mask.Based on feedback from the Allen Institute.
Checklist
flake8from the source directory.#XXXnotation whereXXXis the issue number?